Fix/svg mime type global registration#296
Conversation
Adds global upload_mimes filter registration on init to ensure SVG MIME type is recognized in all contexts, fixing compatibility with WP Offload Media and similar plugins. Fixes 10up#286
| add_action( 'init', array( $this, 'register_svg_mime_type' ) ); | ||
| add_filter( 'wp_check_filetype_and_ext', array( $this, 'fix_mime_type_svg' ), 100, 4 ); |
There was a problem hiding this comment.
The concern with adding these hooks is that once SVGs are globally allowed via upload_mimes, WordPress will accept SVG uploads regardless of whether they pass through Safe SVG’s sanitizer.
In core, SVG uploads are typically validated because most upload paths go through wp_handle_upload() or wp_handle_sideload(), which we explicitly hook into via wp_handle_upload_prefilter and wp_handle_sideload_prefilter.
However, both of those functions are thin wrappers around _wp_handle_upload(), and that lower-level function can be called directly with any $action value (see core code).
When _wp_handle_upload() is called with a custom action (for example 'wp_upload_test'), the resulting prefilter becomes:
wp_upload_test_prefilter
Safe SVG does not hook into arbitrary variations of this filter. We intentionally only hook into the core upload and sideload prefilters.
If SVGs are globally permitted at the MIME level, and a plugin or theme calls _wp_handle_upload() with a non-standard action, that SVG upload will be accepted without ever running through the sanitizer. This creates a potential security vulnerability.
The key issue here is that we cannot assume third-party plugins will follow WordPress core’s upload flow. Nothing enforces that, and many plugins already call _wp_handle_upload() directly. Allowing SVGs globally means we implicitly trust all of those callers, which Safe SVG should not do.
Historically, this is why these hooks were removed in #228.
Regarding WP Offload Media: if it needs SVG MIME support, a safer approach would be one of the following:
- Provide a temporary or scoped filter that allows SVG MIME types only during a controlled upload flow
- Require integrations to explicitly opt in or add their own MIME handling, rather than enabling SVGs globally
Globally enabling SVG MIME types without guaranteeing sanitizer coverage across all upload paths is not something we can safely do.
There was a problem hiding this comment.
Good points. Thanks for the thorough review. The readme updates at least give everyone a heads up so they can adjust appropriately.
|
Closing in favor of #302. |
Safe SVG PR Template (Issue #286)
Description of the Change
Problem:
Safe SVG versions 2.2+ can be incompatible with WP Offload Media (and potentially other plugins that process media outside of standard WordPress admin page loads). When WP Offload Media attempts to offload SVG files to S3-compatible storage, it may fail with errors like:
AS3CF: Mime type "" is not allowed (Media Library Item with id XXX)ContentType must be a string... Found bool(false)Root Cause:
Safe SVG currently adds SVG MIME detection support contextually—during specific admin page loads (e.g.
load-upload.php,load-post.php,load-site-editor.php, etc.) viaallow_svg_from_upload(), which temporarily registers awp_check_filetype_and_extfilter at priority 75, then removes it after upload processing completes.When third-party plugins call
wp_check_filetype_and_ext()outside of these contexts (e.g. WP Offload Media's background offload processing, cron jobs, REST API calls, CLI operations), Safe SVG's filter has been removed, causing MIME type detection for SVGs to return an empty type. This breaks downstream behavior that relies on a valid MIME type.Solution:
Add a persistent
wp_check_filetype_and_extfilter at priority 100 that ensures SVG MIME detection works in all contexts—including after Safe SVG's contextual priority-75 filter is removed during upload cleanup.This change:
__construct()and never removes itpre_move_uploaded_filecleanupwp_check_filetype_and_ext()always returnsimage/svg+xmlfor.svgand.svgzfilesWhy this is safe:
check_for_svg()continues to sanitize during upload with capability checksWhat WP Offload Media users still need to do:
This fix ensures WordPress correctly identifies SVG MIME types. However, WP Offload Media maintains its own allowed MIME types list (defaulting to WordPress'
get_allowed_mime_types()), which may not include SVG depending on your site configuration.Users experiencing
AS3CF: Mime type "image/svg+xml" is not allowederrors after applying this fix will need to add a filter to their theme or mu-plugin:This is expected behavior—WP Offload Media intentionally controls which file types are allowed for offload independent of WordPress upload capabilities, and this is outside Safe SVG's scope.
Closes #286
How to test the Change
Basic MIME detection test:
WP Offload Media integration test:
as3cf_allowed_mime_typesfilter (see "What WP Offload Media users still need to do" above).Expected behavior without the
as3cf_allowed_mime_typesfilter:AS3CF: Mime type "image/svg+xml" is not allowedChangelog Entry
Credits
Props @oneprince
Checklist: